Refactor - JIT noop insertion #584
Conversation
…ad of gen_range().
Lichtso
force-pushed
the
refactor/jit-noop-insertion
branch
from
6b58569 to
31173a5
Compare
Lichtso
force-pushed
the
refactor/jit-noop-insertion
branch
from
31173a5 to
ef1fe8d
Compare
| @@ -321,6 +325,7 @@ pub struct JitCompiler<'a, C: ContextObject> { | |||
| pc: usize, | |||
| last_instruction_meter_validation_pc: usize, | |||
| next_noop_insertion: u32, | |||
| noop_range: Uniform<u32>, | |||
| runtime_environment_key: i32, | |||
| diversification_rng: SmallRng, | |||
There was a problem hiding this comment.
Not related to this PR, but I found out that SmallRng isn't good when security is a concern:
Furthermore, SmallRng is not a good choice when:
Security against prediction is important. Use StdRng instead.
There was a problem hiding this comment.
The PRNG here needs to be fast, not high quality. The actual security comes from diversity of seeds in validators across the cluster.
| @@ -393,7 +399,7 @@ impl<'a, C: ContextObject> JitCompiler<'a, C> { | |||
| self.emit_subroutines(); | |||
|
|
|||
| while self.pc * ebpf::INSN_SIZE < self.program.len() { | |||
| if self.offset_in_text_section + MAX_MACHINE_CODE_LENGTH_PER_INSTRUCTION > self.result.text_section.len() { | |||
| if self.offset_in_text_section + MAX_MACHINE_CODE_LENGTH_PER_INSTRUCTION * 2 >= self.result.text_section.len() { | |||
There was a problem hiding this comment.
I think this overestimation is too high. We could decrease MAX_MACHINE_CODE_LENGTH_PER_INSTRUCTION to 90. Also, a nop instruction is a single byte. If we emit a maximum of (e.g.) 8 x86 instructions per SBPF instruction, we could do MAX_MACHINE_CODE_LENGTH_PER_INSTRUCTION + 8. In the worst case scenario, there will be one nop per x86 instruction.
There was a problem hiding this comment.
It is high yes, but it can only run into the end one sBPF instruction earlier. And that should be covered by MAX_EMPTY_PROGRAM_MACHINE_CODE_LENGTH. Generally I would be rather conservative and overestimate here as this is the only bounds-check for the emitted code size.
No description provided.